Skip to content

eventloop: don't conclude on an unreadable event count - #5

Open
WarpRomo wants to merge 1 commit into
google-deepmind:mainfrom
WarpRomo:fix/eventloop-conclude-on-unread-event-count
Open

eventloop: don't conclude on an unreadable event count#5
WarpRomo wants to merge 1 commit into
google-deepmind:mainfrom
WarpRomo:fix/eventloop-conclude-on-unread-event-count

Conversation

@WarpRomo

@WarpRomo WarpRomo commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

The during-generation event count discarded its error, so a failed read was indistinguishable from an empty queue and the agent concluded on it.

Why

An autonomous agent ending a turn with no tool calls checks whether events arrived while the model was generating, then concludes. That check dropped GetEventCount's error, so a failed read became a count of zero.

Concluding is terminal. db.IsSpine excludes concluded, so RecoverRun never resumes the session and an environment notification cannot revive it. Anything queued at the bumped step is dropped, and the parent gets a concluded verdict for work that never finished.

Shutdown reaches it. session.Registry.CancelAll only ctx-cancels and leaves DB status alone so RecoverRun can resume. A cancel landing between FinalizeStep and the check makes GetEventCount return context.Canceled, and conclude() writes with context.Background(), so the dead context does not stop the terminal write. A transient store read failure gives the same zero.

Implementation

  • check GetEventCount's error instead of discarding it
  • route it through recordFailure, as every other store call in loop does
  • a cancelled ctx returns without writing; a db.ErrStore error leaves the session ongoing-but-dead for Recover

Validation

Interrupted headless runs

amplio headless run against deepseek-v4-flash, interrupted with SIGINT (what Ctrl-C sends) at a swept delay after the model's completion reaches the agent. Identical invocation and identical interrupt schedule on both binaries, so only the patch differs. 27 runs, 3 per delay, final status read from each run's sqlite DB.

delay_us    upstream dd403f3    patched        n
      0     ongoing             ongoing      3/3
    100     ongoing             ongoing      3/3
    250     ongoing             ongoing      3/3
    500     ongoing             ongoing      3/3
   1000     ongoing             ongoing      3/3
   2000     ongoing             ongoing      3/3
   5000     concluded           ongoing      2/3   <- window
   5000     ongoing             ongoing      1/3
  10000     concluded           concluded    3/3
  20000     concluded           concluded    3/3

Below 5 ms the interrupt arrives before the check, FinalizeStep fails, and both binaries leave the session ongoing. At 10 ms and above the run has already concluded on its own. At 5 ms the interrupt lands inside the window and the two diverge:

UPSTREAM  dd403f3    final session status: concluded
  INFO  interrupted, cancelling run
  (no further agent output; db.IsSpine excludes concluded, so RecoverRun
   would never resume this run)

PATCHED   c826171    final session status: ongoing
  INFO  interrupted, cancelling run
  ERROR agent failure session=main-agent
        error="count events arrived during generation: db store: context canceled"

An uninterrupted control run on the same setup concludes normally, so the divergence comes from the interrupt rather than the harness.

Tests

Two regression tests, one per cause, fail on dd403f3 and pass with the patch:

go test ./internal/agent/eventloop \
  -run 'TestEventLoop_(ShutdownAtConclusionCheckLeavesSessionRecoverable|StoreErrorAtConclusionCheckLeavesSessionRecoverable)' \
  -count=1 -v

BEFORE   --- FAIL: TestEventLoop_ShutdownAtConclusionCheckLeavesSessionRecoverable
         --- FAIL: TestEventLoop_StoreErrorAtConclusionCheckLeavesSessionRecoverable
AFTER    --- PASS: TestEventLoop_ShutdownAtConclusionCheckLeavesSessionRecoverable
         --- PASS: TestEventLoop_StoreErrorAtConclusionCheckLeavesSessionRecoverable

Focused x50 and -race x20 clean; full tests, vet, lint and build pass on go1.26.2.

The shutdown test forces a real interleaving against the real sqlite store, with nothing stubbed and no error injected. The store-error test does inject its error, noted as such in the file, to cover the db.ErrStore branch that the shutdown case does not reach.

Before concluding a bare no-tool turn, the loop checks whether events
arrived while the model was generating. That check discarded
GetEventCount's error, so a failed read became a count of zero and was
indistinguishable from "nothing arrived".

Concluding on it is terminal and unrecoverable: db.IsSpine excludes
concluded, so RecoverRun skips the session permanently, and an environment
notification does not revive a finished session either. Anything queued at
the bumped step -- a child_result(crashed), operator input, a background
job reporting in -- is lost with it, and the parent is notified with a
concluded verdict for work that never finished.

Two causes are reachable. On shutdown, session.Registry.CancelAll only
ctx-cancels and deliberately leaves DB status alone so RecoverRun can
resume; a session in the window between FinalizeStep and this check reads
context.Canceled here, and conclude() writes with context.Background(), so
the dead context does not stop the terminal write. Separately, a transient
store read failure yields the same zero while the following write still
succeeds.

Route the error through recordFailure, which already classifies both cases:
a cancelled ctx returns without writing and leaves the status for recovery,
and a db.ErrStore error leaves the session ongoing-but-dead for Recover.
Every other store call in the loop already does this; this was the only one
that did not, and it sits immediately before the irreversible transition.

Both causes are covered by regression tests that fail on the parent commit.
@WarpRomo
WarpRomo force-pushed the fix/eventloop-conclude-on-unread-event-count branch from 9e47228 to c826171 Compare September 6, 2026 10:27
@WarpRomo
WarpRomo marked this pull request as ready for review September 6, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant